Skip to content

test: add memory tests (via DHAT) #1009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Oct 3, 2025
Merged

test: add memory tests (via DHAT) #1009

merged 4 commits into from
Oct 3, 2025

Conversation

@zachschuermann
Copy link
Member

@zachschuermann zachschuermann commented Jun 5, 2025 •
edited
Loading

What changes are proposed in this pull request?

exploring DHAT tests for memory profiling. this adds two ignored tests in a separate (new) mem-test internal crate.

run with:

cargo test -p mem-test -- --ignored --nocapture

How was this change tested?

test-only

@zachschuermann zachschuermann changed the title [WIP] test: add DHAT tests test: add DHAT tests Sep 18, 2025
@codecov
Copy link

codecov bot commented Sep 18, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.83%. Comparing base (39c440d) to head (e77caac).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1009   +/-   ##
=======================================
  Coverage   84.83%   84.83%           
=======================================
  Files         113      113           
  Lines       28735    28735           
  Branches    28735    28735           
=======================================
  Hits        24376    24376           
  Misses       3203     3203           
  Partials     1156     1156

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added the breaking-change Change that require a major version bump label Sep 18, 2025
@zachschuermann zachschuermann changed the title test: add DHAT tests test: add memory tests (via DHAT) Sep 19, 2025
@zachschuermann zachschuermann marked this pull request as ready for review September 19, 2025 19:09
@zachschuermann zachschuermann removed the breaking-change Change that require a major version bump label Sep 19, 2025
scovich
scovich reviewed Sep 26, 2025
View reviewed changes
Copy link
Collaborator

@scovich scovich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I know how DHAT works... can you given an example of expected output in the PR description?

OussamaSaoudi
OussamaSaoudi approved these changes Sep 26, 2025
View reviewed changes
Copy link
Collaborator

@OussamaSaoudi OussamaSaoudi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems useful. We can iterate as needed.

mem-test/tests/dhat_large_table_log.rs

#[ignore = "mem-test - run manually"]
#[test]
fn test_dhat_large_table_log() -> Result<(), Box<dyn std::error::Error>> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof

Failed to get latest snapshot: ObjectStore(Generic { store: "LocalFileSystem", source: UnableToOpenFile { source: Os { code: 24, kind: Uncategorized, message: "Too many open files" }, path: "/var/folders/cr/cl0ybfwx6kjd8bybfc2mr4rw0000gp/T/.tmpapqcBg/_delta_log/00000000000000000749.json" } })

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah macos? i think might need to bump up ulimit

@OussamaSaoudi
Copy link
Collaborator

@scovich Here's the output that didn't crash:

   Running tests/dhat_large_table_data.rs (target/debug/deps/dhat_large_table_data-55715a37bc576271)

running 1 test
File size (compressed file size):    1073334219 bytes
Total size (uncompressed file size): 1073183367 bytes
Heap stats after writing parquet:
HeapStats { total_blocks: 3939016, total_bytes: 7698447898, curr_blocks: 3, curr_bytes: 88, max_blocks: 6245, max_bytes: 2237041899 }
Heap stats after creating snapshot:
HeapStats { total_blocks: 3940234, total_bytes: 7700146377, curr_blocks: 378, curr_bytes: 70344, max_blocks: 6245, max_bytes: 2237041899 }
Heap stats after building scan:
HeapStats { total_blocks: 3940250, total_bytes: 7700147415, curr_blocks: 390, curr_bytes: 71080, max_blocks: 6245, max_bytes: 2237041899 }
Heap stats after scan execution:
HeapStats { total_blocks: 3979394, total_bytes: 10933070181, curr_blocks: 764, curr_bytes: 98934, max_blocks: 6245, max_bytes: 2237041899 }
Total rows read: 1000000
test test_dhat_large_table_data ... ok

nicklan
nicklan approved these changes Oct 2, 2025
View reviewed changes
Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved for now as this is purely additive and we can iterate on exactly what we want to do with them

@zachschuermann zachschuermann merged commit 9eee114 into delta-io:main Oct 3, 2025
21 checks passed
@zachschuermann zachschuermann deleted the dhat-tests branch October 3, 2025 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scovich scovich scovich left review comments

@nicklan nicklan nicklan approved these changes

@OussamaSaoudi OussamaSaoudi OussamaSaoudi approved these changes

Assignees

@zachschuermann zachschuermann

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zachschuermann @OussamaSaoudi @nicklan @scovich